Dataset match subdomain/v1 - #15058
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #15058 +/- ##
==========================================
- Coverage 82.59% 82.58% -0.01%
==========================================
Files 990 990
Lines 271761 271797 +36
==========================================
+ Hits 224465 224468 +3
- Misses 47296 47329 +33
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
| Dataset *set; | ||
| uint8_t cmd; | ||
| DatasetFormats format; | ||
| bool match_subdomain; /* walk up domain hierarchy on lookup */ |
There was a problem hiding this comment.
nit: placing bool after uint8_t leads to a more efficient memory layout
| { | ||
| for (uint32_t i = 1; i < data_len; i++) { | ||
| if (data[i] == '.') { | ||
| int r = DatasetLookup(set, data + i, data_len - i); |
There was a problem hiding this comment.
Wonder if we should limit the number of calls to DatasetLookup in case someone tricks an input like .............................. many times
There was a problem hiding this comment.
We could skip consecutive dots, then .............................. will have 0 lookups. But it wouldn't cover the case where the domain is like .a.a.a.a.a.a.a.a.a.a.a..... DNS names max at 253 bytes, so worst case is ~126 lookups. Is this fine? What do you think?
There was a problem hiding this comment.
we can use this for buffers that come from other data, like http.host where there isn't necessarily a length limit enforced
There was a problem hiding this comment.
Good point, I'll add a skip for consecutive dots and a cap for the lookups. What's a reasonable cap?
catenacyber
left a comment
catenacyber
left a comment
There was a problem hiding this comment.
Thanks for the work, looks good
Needs clang format fix apparently
Also the commit messages need to reference the ticket number
|
And this also needs SV tests |
|
v2: #15073 |
4 participants
Contribution style:
https://docs.suricata.io/en/latest/devguide/contributing/contribution-process.html
Our Contribution agreements:
https://suricata.io/about/contribution-agreement/ (note: this is only required once)
Changes (if applicable):
Link to ticket: https://redmine.openinfosecfoundation.org/issues/8385
Describe changes:
Provide values to any of the below to override the defaults.
link to the pull request in the respective
_BRANCHvariable.SV_REPO=
SV_BRANCH=
SU_REPO=
SU_BRANCH=